Skip to content

Improve filter_extremes methods in Dictionary and HashDictionary#2303

Merged
menshikh-iv merged 1 commit into
piskvorky:developfrom
horpto:dictionary-keep_ids
Jan 9, 2019
Merged

Improve filter_extremes methods in Dictionary and HashDictionary#2303
menshikh-iv merged 1 commit into
piskvorky:developfrom
horpto:dictionary-keep_ids

Conversation

@horpto
Copy link
Copy Markdown
Contributor

@horpto horpto commented Dec 19, 2018

  • use search by set instead of list
  • refine default value of dict.get() method
  • inplace sort

- use search by set instead of list
- refine default value of dict.get() method
- inplace sort
@horpto horpto force-pushed the dictionary-keep_ids branch from 19b3ba1 to f0d0621 Compare December 19, 2018 06:01
)
good_ids = sorted(good_ids, key=lambda x: self.num_docs if x in keep_ids else self.dfs.get(x), reverse=True)
]
good_ids.sort(key=lambda x: self.num_docs if x in keep_ids else self.dfs.get(x, 0), reverse=True)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why sorted(generator) -> list.sort()? Is this save memory or what's a difference?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves me from linter XD
No, it does not save memory https://github.com/python/cpython/blob/master/Python/bltinmodule.c#L2201

@menshikh-iv menshikh-iv changed the title Improve filter_extremes methods in Dictionary and HashDictionary Improve filter_extremes methods in Dictionary and HashDictionary Jan 9, 2019
@menshikh-iv
Copy link
Copy Markdown
Contributor

Thanks @horpto

@menshikh-iv menshikh-iv merged commit 7cbf715 into piskvorky:develop Jan 9, 2019
@piskvorky
Copy link
Copy Markdown
Owner

Awesome! These are the kind of deep and small but important fixes that help us so much. 👍

@horpto horpto deleted the dictionary-keep_ids branch January 19, 2019 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants